Skip catalog loading for the version command#3146
Conversation
geruh
left a comment
geruh
left a comment
There was a problem hiding this comment.
Hey @jx2lee thanks for raising this. I think it makes sense to fix because a version sub command is a standard check. I took a quick pass at this and it seems like this is scoped down to missing and misconfiguration catlog setups as you mentioned.
But this also got me thinking, most users today are used to first running x --version from the cli. By moving to use clicks' click.version_option we can enable pyiceberg --version and wouldn't have to account for it in the sub commands.
Also, it's worth mentioning that a user running the --version command today will get:
> pyiceberg --version
Error: No such option: --version. Did you mean --verbose? But kind of thinking out loud here as this is small lol.
cc: @kevinjqliu
|
@geruh Thanks for the review.
I agree that For this PR, I made the fix without removing the existing @kevinjqliu What do you think? |
2 participants
Rationale for this change
pyiceberg versionshould print the installed PyIceberg version even when.pyiceberg.yamlor catalog-related environment variables are invalid. CLI group callback eagerly loads the catalog for every subcommand, soversioncan fail before it prints anything.This change skips catalog loading for the
versionsubcommand and adds a regression test for that behavior.Are these changes tested?
Yes.
test_version_does_not_load_catalogintests/cli/test_console.pyAre there any user-facing changes?
Yes.
pyiceberg versionnow prints the version without requiring valid catalog configuration.